In theory, those paths are generated using rebar3, but it seems for the case of tests, it was not correctly done.

If I understand correctly, this function is reading the path from the client request, and will return the kind of limiter used. Using a data structure like a proplist or a map can probably be easier to modify in the future. Not sure if could be easy to do right now though.

The problem I see here is, if we are adding a new path, we will need to add it in this function (at least if we need a special rate limit for it).

Yes, it's certainly going to some work to add a new rate limiter group.

1. add Rate Limiter Group (RLG) process to arweave_limiter_sup
2. add Rate Limiter Group config parameters as fields to arweave_config config record.
   2b) add defaults for the parameters.
3. add Rate Limiter routing logic to ar_http_iface_rate_limiter_middleware

IMHO, this mapping function is a quite idiomatic way to map paths to atoms. (No dynamically named atoms, no need for lookups)

• I'd debate that 2) is a bigger burden than 3) (extending this function).
• I'd like to point out: this PR doesn't aim to introduce dynamic RLGs.
• With a different approach on how we deal with the configuration, I think we can introduce RLGs started in a dynamic manner in the future, if necessary.

We are not adding a lot of new end-point, but perhaps documenting this procedure somewhere could be nice.

To be honest I didn't realize we had a disable blacklist option, and I'm not sure if it is currently in use. It looks like it would completely disable rate limiting on the server side. @ldmberman @humaite are you aware of any nodes that use it?

It's possible some large miners use it on internal nodes used to server data to mining nodes (for example). But I'm not sure it would even be that effective since by default clients will self-throttle regardless of the server-side behavior.

This is a long way of asking:

1. @khetzl did you mean to remove this option? (e.g. because you've replaced it with another configuration option)
2. Even if the removal was unintentional... I'm not sure we actually need or want this particular flag. In general the enable and disable options are poorly understood and rarely used, so if we want to disable server-side rate limiting a more formal config is a better choice. Only question is whether anyone is using the current flag?

I intended to remove this, we shouldn't allow unrestricted access to resources to the public.
Trusted peers can be listed so their requests are limited via a less restrictive Rate Limiter Group. (Currently, no limiting at all)

Note: Even if the disable blacklist flag is enabled in the current release, a global concurrency limit is enforced by cowboy/ranch.

Got it. I agree it's correct to remove. However worth providing some context:

• In cases like this where it is the right call to remove an option, we may still need to flag and communicate the removal for any miners that are relying on it. Both in the release notes and in Discord. I suspect this option isn't used much (or at all) so probably fine to silently remove.
• Many miners implement their own security in front of their node. Specifically they will run nodes on a LAN without direct internet connection - so even if they opt in to using the disable blacklist flag it doesn't necessarily mean they or we are providing unrestricted access to the public.
• I don't think this flag is used, but if it is used it is likely due to the above. We've had a few miners run large networks with many internal (non publicly accessible nodes). In the past they tried using local_peers but since arweave doesn't currently support any sort of wildcards/subnetting, and because the ndoe shutdown/boot time can take a long time, some of the miners had trouble managing rate limiting for their internal nodes. It's in that scenario where I could see disable blacklist being useful (setup an internal data-sharing node, disable blacklist, and then let all your other nodes connect to without worrying about needing to restart periodically to update local_peers)

All that said: I do think fine to remove. But I wanted to call out that context as our operating model is often different than that of non-blockchain companies (e.g. we build infrastructure software to be run by 3rd parties as well as by us internally - and those 3rd parties vary from novice to enterprise-level)

@khetzl @humaite What's the recommended approach for the new config system regarding how deep to nest configs?

E.g. Is there a clear preference http_api_limiter.metrics.sliding_window_timestamp_cleanup_interval vs. http_api_limiter.metrics.sliding_window.timestamp_cleanup_interval?

Especially in a JSON format I could see grouping all the sliding_window or leaky_tick options together making it a little easier to keep everything straight.

I have no preference, I'm happy to introduce further grouping, if you guys think it makes more sense.
I tried to keep it as close to what we use internally in the RLG implementation for readability, but . vs _ wouldn't make much of a difference in this sense.

👍 I don't feel strongly either. We can defer to @humaite and if he doesn't have a clear preference, leave as is.

Nice. I like this approach - really helps with modularization 👍

Where is this called? I only see it called from ar_http_iface_middleware:handle_post_tx_accepted. I was expecting to see it called whenever an inbound HTTP request is processed, but perhaps I misunderstood

Ah yeah looks like I misunderstood. Reading the comment reduce_for_peer is a special case so that we don't penalize peers for sharing valid transactions with us. The normal leaky bucket and window counter tracking is handled elswhere.

Maybe add a comment to this function to describe it? Main relevant point that I see is that this function is not the standard way to reduce the request count for a peer, but is an edge case handler.

Yes, that's correct. The previous solution immediately reduces when a transaction is accepted, to deal with high load.
I'd like to point out: this is a double reduction, we are just following the previous behaviour and should be rather handled with correct set up of config parameters.

Or some other comment just to explain the double-reverse

Without buckets we can still get an average over time because the histogram includes xxx_count and xxx_sum fields. Most of our internal charts just use this average over time value to track performance changes. But if some degree of bucketing would be helpful for this metric (e.g. if the distribution is skewed per timeslice), defintely add them!

We have a broader issue of how to deal with our growing number of metrics, but I think we'll likely need to tackle that holistically - until then feel free to add whatever metrics you think are necessary

Timers not canceled on termination causing resource leak

Medium Severity

The terminate/2 callback ignores the state and doesn't cancel the timers created by timer:send_interval in init/1. Timer refs are stored in state as leaky_tick_timer_ref and timestamp_cleanup_timer_ref, but when the process terminates (crash, restart, shutdown), these timers remain active in the global timer server. They continue firing indefinitely, sending messages to a dead pid. Each process restart leaks two timer entries that accumulate over time, consuming memory and CPU in the timer server.

Yeah, he's commenting this over and over again, but cancelling would crash the tests, so we are not doing it. these processes should live for the lifetime of the node anyway.

Maybe if you resolve all instances of the comment it will stop?

Oh looks like a known issue: https://forum.cursor.com/t/bugbot-should-filter-out-previously-resolved-dismissed-issues/147748

Test expects wrong error count for rate limiting

Low Severity

In test_node_blacklisting_get_spammer, the test sends exactly sliding_window_limit + leaky_limit (300) requests but expects 1 error. Based on the rate limiter logic, all 300 requests should pass: the first 150 via sliding window and the next 150 via leaky bucket tokens. The first rejection occurs at request 301. The test only passes because the tolerance check Got >= ExpectedErrors - Tolerance evaluates to Got >= -4, which is always true.

This is incorrect, the test asserts a range correctly

Config validation rejects valid zero sliding window limit

Medium Severity

The config validation for sliding_window_limit options requires Limit > 0, but multiple default values in arweave_config.hrl are set to 0 (e.g., DEFAULT_HTTP_API_LIMITER_DATA_SYNC_RECORD_SLIDING_WINDOW_LIMIT, DEFAULT_HTTP_API_LIMITER_BLOCK_INDEX_SLIDING_WINDOW_LIMIT, etc.). A value of 0 is valid and means "skip sliding window, use only leaky bucket." Users cannot explicitly configure this behavior via the config file since validation would reject it with a bad_value error.

Histogram buckets misconfigured for microsecond measurements

Medium Severity

The histogram ar_limiter_response_time_microseconds has buckets [0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10, 50] but timer:tc/2 returns values in microseconds. A typical gen_server call takes 50-500+ microseconds, meaning virtually all observations will overflow into the +Inf bucket, rendering the histogram useless for monitoring and debugging. The buckets need to be scaled appropriately for microsecond values (e.g., [10, 50, 100, 500, 1000, 5000, 10000, 50000]).

Missing config option to disable rate limiting globally

Medium Severity

The old ar_blacklist_middleware allowed users to disable rate limiting by adding "blacklist" to the disable config list. The new ar_http_iface_rate_limiter_middleware doesn't check Config#config.disable for this option. Users who configured "disable": ["blacklist"] to bypass rate limiting will unexpectedly start receiving 429 errors after upgrading, as this configuration is silently ignored by the new middleware.

Discussed above

Histogram buckets mismatch with microsecond measurement unit

Medium Severity

The ar_limiter_response_time_microseconds histogram has buckets [0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10, 50], but timer:tc/2 returns time as integer microseconds. A typical gen_server call taking 100 microseconds would return Time = 100, which exceeds the highest bucket (50), causing all observations to fall only in the +Inf bucket. The buckets appear designed for seconds rather than microseconds, making the histogram distribution data useless for observability.

Interval timers not cancelled on gen_server termination

Medium Severity

The terminate/2 callback matches on leaky_tick_timer_ref and timestamp_cleanup_timer_ref but doesn't call timer:cancel/1 on them. Timers created with timer:send_interval/3 persist in the timer server until explicitly cancelled. When the gen_server terminates (due to crash, restart, or stop), these timers continue firing and accumulate as resource leaks. Other modules in the codebase properly cancel timers during cleanup.

Peer limiter not reset causing inconsistent test state

Low Severity

The reset_node/0 function resets the local arweave_limiter_sup but only resets the peer's ar_blacklist_middleware, not the peer's limiter. This creates an asymmetric reset where the local node has a clean limiter state but peer1 retains its previous limiter state, potentially causing test flakiness or incorrect rate limiting behavior when tests interact with the peer.